Skip to content

Close two test gaps: unverified prebuild Info.plist and a missing .node fixture - #448

Closed
kraenhansen wants to merge 1 commit into
nextfrom
claude/sharp-fermi-rgb4ex
Closed

Close two test gaps: unverified prebuild Info.plist and a missing .node fixture#448
kraenhansen wants to merge 1 commit into
nextfrom
claude/sharp-fermi-rgb4ex

Conversation

@kraenhansen

@kraenhansen kraenhansen commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Closes #424.

verify-prebuilds now reads the Info.plist it finds

verifyApplePrebuild used to continue past Info.plist without reading it. It now parses it with @expo/plist (already a dependency elsewhere in the repo) and asserts:

  • CFBundleExecutable matches the framework's library name (the .framework directory's basename)
  • CFBundleIdentifier matches escapeBundleIdentifier("com.callstackincubator.node-api.<libraryName>") — the default writeFrameworkInfoPlist (packages/host/src/node/prebuilds/apple.ts) writes when no --apple-bundle-identifier is passed, which is how node-addon-examples builds

escapeBundleIdentifier is now exported from the package's public node entrypoint so the verify script (a react-native-node-api consumer, same as any addon author) can import it rather than re-deriving the escaping rule.

The Babel plugin test now proves its own claim — and that found a real bug

"and does not touch required JS files" asserted the plugin leaves require('./my-addon') alone, but the fixture had no my-addon.*.node for the plugin to have found in the first place — the assertion passed for the wrong reason. Adding a sibling my-addon.apple.node (as the TODO asked) makes the test exercise real precedence between a .js file and a same-named addon, and it failed: isNodeApiModule didn't check whether a same-named .js/.cjs/.mjs/.json file would already satisfy require() before considering .node prebuilds, so the plugin rewrote a call that Node's own resolution would never route to the addon.

Fixed isNodeApiModule to defer to a colliding source file when the module path has no explicit .node extension, matching Node's own resolution order (.js/.json before .node). Explicit require('./my-addon.node') calls are untouched by this check, since there's no resolution ambiguity to defer to.

Verification

  • pnpm run build, pnpm run lint, pnpm exec prettier --check clean
  • packages/host's test suite: the plugin and escapeBundleIdentifier suites pass in full; the only failures are 4 pre-existing permission-based tests that fail in this sandbox because it runs as root (verified they fail identically on next before this change)
  • Manually exercised verifyFrameworkInfoPlist against a real Info.plist written by createAppleFramework (install_name_tool isn't available on Linux, so I read the file straight after it's written) — both the pass and the mismatch-detection path behave as expected
  • No changeset for @react-native-node-api/node-addon-examples: it's private

cc @kraenhansen for review.

…de fixture

verify-prebuilds.mts now parses each Apple framework's Info.plist and
asserts CFBundleExecutable/CFBundleIdentifier match what
writeFrameworkInfoPlist wrote, instead of skipping the file.

The Babel plugin test for "does not touch required JS files" now includes
a sibling my-addon.apple.node next to my-addon.js, so the assertion is
exercised rather than vacuously true. That exposed a real bug:
isNodeApiModule didn't check whether a same-named .js/.cjs/.mjs/.json file
would already satisfy the require() before ever considering a .node
prebuild, so the plugin could rewrite a require() call that Node's own
resolution would never route to the addon. Fixed to defer to a colliding
source file, matching Node's own module resolution order.

Closes #424

Copy link
Copy Markdown
Collaborator Author

Closing this in favor of #450, which builds on @wanxiankai's earlier #426 (the first PR to fix #424) plus the same improvements this PR made (moving the fix into isNodeApiModule, zod validation, reusing escapeBundleIdentifier). Sorry for the duplicated effort — #426 wasn't spotted before this PR was opened.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Two test gaps: unverified prebuild Info.plist and a missing .node fixture in the Babel plugin tests

2 participants